Skip to content

Remove duplicate run call for ApplicationRunner#1887

Merged
oberstet merged 4 commits into
crossbario:masterfrom
JavierSab:patch-1
Jun 25, 2026
Merged

Remove duplicate run call for ApplicationRunner#1887
oberstet merged 4 commits into
crossbario:masterfrom
JavierSab:patch-1

Conversation

@JavierSab

@JavierSab JavierSab commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Removed duplicate runner.run(Component) call. In python wamp pubsub basic example.

Description

Removal of a duplicate run() line in one of the WAMP simple pubsub client examples.


Related Issue(s)

Closes or relates to #


Checklist

  • I have referenced relevant issue numbers above
  • I have performed a self-review of my code and it follows
    the style guidelines of this project
  • I have added new or used existing tests that prove my fix
    is effective or that my feature works
  • I have added necessary documentation (if appropriate) and
    updated the changelog
  • I have added an AI assistance disclosure file (required!)
    in this PR

AI Assistance Disclosure File

⚠️ Required for this PR: You MUST include a disclosure
file at .audit/<branch-name>.md right in this PR. The
disclosure file must follow the exact format and content as
described below. Your PR will not be accepted without a
disclosure file.

Example 1 file contents of your disclosure file
.audit/<branch-name>.md:

## AI Assistance Disclosure

- [ ] I did **not** use any AI-assistance tools to help create this pull request.
- [x] I **did** use AI-assistance tools to *help* create this pull request.
- [x] I have read, understood and followed the projects' [AI Policy](https://github.com/crossbario/autobahn-python/blob/main/AI_POLICY.md) when creating code, documentation etc. for this pull request.

Submitted by: @your-github-username
Date: YYYY-MM-DD
Related issue(s): #issue-number
Branch: branch-name

OR

Example 2 file contents of your disclosure file
.audit/<branch-name>.md:

## AI Assistance Disclosure

- [x] I did **not** use any AI-assistance tools to help create this pull request.
- [ ] I **did** use AI-assistance tools to *help* create this pull request.
- [x] I have read, understood and followed the projects' [AI Policy](https://github.com/crossbario/autobahn-python/blob/main/AI_POLICY.md) when creating code, documentation etc. for this pull request.

Submitted by: @your-github-username
Date: YYYY-MM-DD
Related issue(s): #issue-number
Branch: branch-name

Submitting code generated primarily by AI, or for which you
cannot claim human authorship, is not permitted. See
AI Policy
for details.

Example 1 OR Example 2 show the only valid two variants. You
cannot have both or none of the first two marks checked, and you
must always have the last tick checked.

Well, "must" if you want your PR to be accepted and ultimately
merged that is. Of course you are always free to
Go ahead! Fork my Day. (TM) ;) This is Open-source.

Removed duplicate runner.run(Component) call. In python wamp pubsub basic example.
@oberstet

oberstet commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Thanks for this, @JavierSab — nice catch! 🙌 The change is correct: that second
runner.run(Component) in the example is dead/duplicate code, and removing it is
exactly right. Much appreciated.

A few small process bits before we can merge. Up front: some of these aren't
spelled out in CONTRIBUTING.md yet — that's on us, not you
— so thanks for
bearing with us while we tidy both your PR and our docs.

So I filed: #1889

And for your specific issue this PR fixes: #1888


  1. We work issue-first. Every change starts from a GitHub issue, and the PR
    references it. To save you time, I just added [BUG] Duplicate runner.run(Component) in Twisted pubsub basic example #1888. It keeps the history/triage tidy.

  2. AI-assistance disclosure as a file (almost there!). Thanks for including
    the disclosure — it does need to live as a file in .audit/ on the branch
    (not only as a PR comment). One small naming tweak: we use
    .audit/<your-github-username>_<branch>.md, so please rename it to
    .audit/JavierSab_patch-1.md (compare the existing files, e.g.
    .audit/bblommers_websocket-server-typing.md). The format/why is described in
    .audit/AI_AUDIT_PROCESS.md. Note that this is required, our CICD checks and will fail if not ..

  3. Branch namepatch-1 is totally fine, no change needed there.

  4. (Optional, nice-to-have) feel free to squash the two commits into one and fix
    the small typo in the audit commit message (ia audit disclossure
    AI audit disclosure). Not a blocker. We will squash-merge to master the PR once accepted anyways!

No tests needed for an example-file cleanup like this. Once the issue reference
and the renamed .audit/JavierSab_patch-1.md are in, we'll let CICD run, and get it merged. Thanks
again for contributing to Autobahn|Python! 🚀

@oberstet

Copy link
Copy Markdown
Contributor

@JavierSab thanks for adding an audit file! I now approved to run CICD, let's see.

because, sorry, there is still a small itch with the audit file contents, it doesn't reference the issue #1888 I added in Related issue(s): - and that would allow to track complete history/triage from within version history.

I still approved to run, since I wanted to give you some "positive success" feedback already, and because I want to know whether our CICD would detect the missing issue reference automatically, and fail! which it might not yet do - which would be something for us to improve then.

@oberstet

oberstet commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

more notes:

1. Rename the audit file (this is what's failing CI ✋)

The Windows CI job is failing at checkout with:

error: invalid path '.audit/JavierSab:patch-1.md'
Error: The process '...git.exe' failed with exit code 128

The cause is the : (colon) in the filename. Colons are a reserved character in Windows filenames (aka Windows sucks, yes, we know that), so git checkout can't write the file there (it's fine on Linux/macOS, which is why only the Windows job trips). It looks like the GitHub owner:branch head-ref label (JavierSab:patch-1) got used as the filename.

The convention is <github-username>_<branch>.md with an underscore, so:

git mv ".audit/JavierSab:patch-1.md" .audit/JavierSab_patch-1.md
git commit -m "Rename audit file to valid cross-platform name"

(compare existing files, e.g. .audit/bblommers_websocket-server-typing.md). That single rename fixes the Windows checkout and matches the naming convention.

2. Reference the issue in the audit file (+ PR)

Now that there's an issue for this (#1888 — thanks for bearing with the
issue-first process), please reference it so the history is fully traceable:

  • In the audit file, set the Related issue(s): line to #1888.
## AI Assistance Disclosure

- [x] I did **not** use any AI-assistance tools to help create this pull request.
- [ ] I **did** use AI-assistance tools to *help* create this pull request.
- [x] I have read, understood and followed the projects' [AI Policy](https://github.com/crossbario/autobahn-python/blob/main/AI_POLICY.md) when creating code, documentation etc. for this pull request.

Submitted by: @JavierSab
Date: 2026-06-19
Related issue(s): #1888
Branch: JavierSab:patch-1

note: the branchname in the file contents uses ":" (colon), and that is fine for all sane OSes, and also for Windows! and it is the correct name, so we use it.

oberstet added a commit to oberstet/autobahn-python that referenced this pull request Jun 25, 2026
…used-type-ignore-comment

ty is run unpinned (we ride the latest release and adjust the ignore
list as it evolves). ty 0.0.53 began emitting two diagnostic classes on
existing, unchanged source that the check-typing recipe did not ignore,
so `ty check` exited 1 and Code Quality Checks went red:

  - 921x possibly-missing-submodule (vendored FlatBuffers-generated code
    under src/autobahn/wamp/gen/** plus a few flatbuffers.* re-export
    sites in message_fbs.py / message.py / protocol.py)
  -   1x unused-type-ignore-comment (src/autobahn/util.py:1062)

0 errors; all warnings. Drift, not a code regression: master was green
on 2026-06-19 with an earlier ty, and went red on a 2026-06-25 run that
pulled ty 0.0.53 (surfaced on the unrelated example-cleanup PR crossbario#1887).

Add both rules to the check-typing --ignore list. Reproduced red->green
locally with ty 0.0.53 against the cpy311 venv: 922 diagnostics/exit 1
before, "All checks passed!"/exit 0 after.

Note: This work was completed with AI assistance (Claude Code).
oberstet added a commit that referenced this pull request Jun 25, 2026
* start new dev branch; add audit file

* Fix #1892: ignore ty 0.0.53 possibly-missing-submodule + unused-type-ignore-comment

ty is run unpinned (we ride the latest release and adjust the ignore
list as it evolves). ty 0.0.53 began emitting two diagnostic classes on
existing, unchanged source that the check-typing recipe did not ignore,
so `ty check` exited 1 and Code Quality Checks went red:

  - 921x possibly-missing-submodule (vendored FlatBuffers-generated code
    under src/autobahn/wamp/gen/** plus a few flatbuffers.* re-export
    sites in message_fbs.py / message.py / protocol.py)
  -   1x unused-type-ignore-comment (src/autobahn/util.py:1062)

0 errors; all warnings. Drift, not a code regression: master was green
on 2026-06-19 with an earlier ty, and went red on a 2026-06-25 run that
pulled ty 0.0.53 (surfaced on the unrelated example-cleanup PR #1887).

Add both rules to the check-typing --ignore list. Reproduced red->green
locally with ty 0.0.53 against the cpy311 venv: 922 diagnostics/exit 1
before, "All checks passed!"/exit 0 after.

Note: This work was completed with AI assistance (Claude Code).
@oberstet oberstet merged commit a66e246 into crossbario:master Jun 25, 2026
37 of 38 checks passed
@oberstet

Copy link
Copy Markdown
Contributor

Merged — thank you, @JavierSab! 🎉 Your fix is in master.


And to close the loop on the one red CI workflow you saw: that failure was entirely on our side, not your change. 🙏 see #1892 and #1893

The failing job was Code Quality Checks → "Code static typing (via ty)", which is completely independent of the example you fixed. We run Astral's ty type checker unpinned (it ships almost daily and we like to ride the latest), and ty 0.0.53 started emitting two new warning classes on our existing, unchanged source.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants